feat: onboarding setup-ruby action - #1
Conversation
7d60602 to
c91eea1
Compare
added subcription checks and author mentions removed added subcription checks and author mentions removed feat: added subscription checks and author updated feat: added subscription checks and author updated
c91eea1 to
cf56612
Compare
Raj-StepSecurity
added
the
review-required
PR ReviewAction TypeNode-based — uses Passed Checks
Failed Checks
Warnings
Security FindingsShell command injection risk — All other code paths examined show no significant security vulnerabilities. SummaryThe onboarding PR is well-structured and passes nearly all required checks. The only failing check is the naming of the audit workflow file ( |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| // Run a cmd command, export its environment, and return new paths | ||
| function exportCommandEnv(command) { | ||
| const cmd = `cmd.exe /s /c "${command} >NUL && "${process.execPath}" -e console.log(JSON.stringify(process.env))"` |
There was a problem hiding this comment.
Security: Shell command injection risk
The command parameter is embedded directly into a cmd.exe /s /c "..." string via template literal without any escaping. If either caller's argument (ridk path or vcVars path) contains shell meta-characters such as ", &, |, or >, the constructed command could execute unintended code.
While the callers currently source these paths from tool-cache paths or vswhere output (not direct user input), a path with embedded special characters — e.g., a tool cache under a directory named with & or " — could break out of the intended string boundary.
Recommendation: Validate that embedded paths contain only expected characters (alphanumeric, \, :, ., space), or use child_process.execFileSync with separate argument arrays instead of a single interpolated string.
// Safer pattern: avoid embedding paths in a raw shell string
// If cmd.exe composition is required, validate that the path matches a safe pattern:
if (!/^[a-zA-Z0-9 :\\._-]+$/.test(ridk)) {
throw new Error(`Unexpected characters in ridk path: ${ridk}`)
}
| common.inputs.selfHosted = inputs['self-hosted'] | ||
| common.inputs.token = inputs['token'] | ||
|
|
||
| process.chdir(inputs['working-directory']) |
There was a problem hiding this comment.
Note: Unsanitized path used in process.chdir()
The working-directory input is passed directly without validation. A value like ../../sensitive-dir would cause the process to change to a directory outside the workspace. This is expected behavior for the action but can be surprising; consider documenting that path traversal is the caller's responsibility.
1 participant
No description provided.